Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature #7

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Feature #7

wants to merge 5 commits into from

Conversation

Ancientwood
Copy link

Thank you for making this theme.

simple append css invert to html dom so if using tag <a> it will refresh otherwise <route-link>
@viko16 viko16 added the enhancement New feature or request label Mar 30, 2020
@viko16 viko16 self-requested a review March 30, 2020 02:02
<div
v-if="filteredList.length === 0"
class="empty-list"
>
Ooops! Nothing here..🙈
落了片白茫茫大地真干净。
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

建议不要直接 hardcode 中文,可以考虑支持下多语言,或者是直接用 $lang 判断

},
plugins: [
'@vuepress/search',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果要加上 Search 插件,不能只给 example 加,要放到顶级的 index.js 里
参考 https://vuepress.vuejs.org/theme/writing-a-theme.html#apply-plugins

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get it.

@viko16
Copy link
Owner

viko16 commented Mar 30, 2020

非常感谢你提交的一些新功能,我本地用过之后觉得很棒。

但在合并之前先提一些建议:

  1. 直接在页面上展示的文本,不要只 hardcode 中文
  2. 翻页似乎没有必要,相当于给用户多了一个点击步骤,建议移除
  3. password 和 hide 的需求似乎很小众,建议交给额外的插件处理,而不是主题内置
  4. nightMode 很棒,但刷新后不会记住用户切换过的模式,可以考虑用 localStorage 记录
  5. 如果要加搜索,不能只给 example 下添加,需要在根目录下 index.js 里追加
  6. 能否对不同功能拆成几个 PR 而不是一次性提交?😂

欢迎继续讨论。

@Ancientwood
Copy link
Author

感谢回复。

翻页或许是见仁见智的,首页列表过长(>50)会影响观感,作为配置项如何,这是否违背了 simple 的初衷?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants